-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore: enhance description of shell_exec #3321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I enhanced the description of shell_exec to provide more contextual information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR need to be updated to fix the issue from root as discussed with @fengju0213
|
Switched non-blocking terminal sessions to export PYTHONUNBUFFERED=1, so Python subprocesses |
|
updated cc @Wendong-Fan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @fengju0213 , for non-blocking mode i think there's some design issue, self._collect_output_until_idle(session_id) stills requires waiting to get initial output, I will enhance this in another PR, feel free to review and check
| env_vars = None | ||
| docker_env = None | ||
| if not block: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check here is not necessary, as code here is already in block case
| env_vars = os.environ.copy() | ||
| env_vars.setdefault("PYTHONUNBUFFERED", "1") | ||
| docker_env = {"PYTHONUNBUFFERED": "1"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compared with providing initial output, would be more clear to return message to let agent know currently it's in non-block mode and the response would be provided later, then content could be viewed by calling shell view?
Description
enhance description of shell_exec for more information
Checklist
Go over all the following points, and put an
xin all the boxes that apply.Fixes #issue-numberin the PR description (required)pyproject.tomlanduv lockIf you are unsure about any of these, don't hesitate to ask. We are here to help!